Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[runtime][refactor] Unify vm and interpreter objects #4693

Merged
merged 4 commits into from
Jan 18, 2020
Merged

[runtime][refactor] Unify vm and interpreter objects #4693

merged 4 commits into from
Jan 18, 2020

Conversation

zhiics
Copy link
Member

@zhiics zhiics commented Jan 13, 2020

This PR:

  • Unifies the Closure objects used by VM and interpreter. Refactored the interpreter to be able to use the same Closure object.

  • Replaces the TupleValue objects with ADT

  • Cleans the vmobj code and makes ADT not specific to the vm namespace.

cc @tqchen @jroesch @wweic @icemelon9

@zhiics zhiics force-pushed the unify_interpreter_vm branch 2 times, most recently from 88c6be4 to 9843014 Compare January 13, 2020 04:24
@zhiics zhiics force-pushed the unify_interpreter_vm branch from 9843014 to e6c8347 Compare January 14, 2020 01:14
@zhiics
Copy link
Member Author

zhiics commented Jan 14, 2020

ping reviewers.

tqchen
tqchen requested changes Jan 15, 2020
View reviewed changes
include/tvm/runtime/common_object.h Outdated Show resolved Hide resolved
@zhiics zhiics force-pushed the unify_interpreter_vm branch 2 times, most recently from 72d2b4c to 1c5d210 Compare January 15, 2020 23:39
@tqchen
Copy link
Member

tqchen commented Jan 16, 2020

would be great if @wweic can also take a look

@wweic
Copy link
Contributor

wweic commented Jan 16, 2020

I'll take a look soon.

jroesch
jroesch reviewed Jan 16, 2020
View reviewed changes
include/tvm/relay/interpreter.h Outdated Show resolved Hide resolved
include/tvm/runtime/vm.h Show resolved Hide resolved
include/tvm/runtime/container.h Outdated Show resolved Hide resolved
src/relay/pass/fold_constant.cc Show resolved Hide resolved
src/runtime/container.cc Outdated Show resolved Hide resolved
tests/python/unittest/test_container.py Show resolved Hide resolved
@jroesch
Copy link
Member

jroesch commented Jan 16, 2020

cc @MarisaKirisame and @slyubomirsky

@zhiics zhiics force-pushed the unify_interpreter_vm branch from 1c5d210 to 040a532 Compare January 16, 2020 22:01
@zhiics
Copy link
Member Author

zhiics commented Jan 16, 2020

@jroesch @tqchen @wweic comments are addressed. PTAL, thanks.

@tqchen
Copy link
Member

tqchen commented Jan 17, 2020 •
edited
Loading

waiting further comments from @jroesch @wweic

wweic
wweic approved these changes Jan 17, 2020
View reviewed changes
Copy link
Contributor

@wweic wweic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. just a small typo

python/tvm/container.py Show resolved Hide resolved
icemelon
icemelon reviewed Jan 17, 2020
View reviewed changes
python/tvm/relay/backend/vm.py Outdated Show resolved Hide resolved
src/relay/backend/interpreter.cc Outdated Show resolved Hide resolved
@zhiics zhiics force-pushed the unify_interpreter_vm branch from 05113a5 to b4347a6 Compare January 17, 2020 19:10
@MarisaKirisame
Copy link
Contributor

Thanks for the change. It now look right.

@icemelon icemelon merged commit acbf885 into apache:master Jan 18, 2020
@icemelon
Copy link
Member

Thanks @zhiics @tqchen @wweic @jroesch @MarisaKirisame . This is now merged.

@zhiics zhiics deleted the unify_interpreter_vm branch January 18, 2020 18:22
This was referenced Jan 19, 2020
Merged
Merged
alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 26, 2020
@zhiics @alexwong
[runtime][refactor] Unify vm and interpreter objects (apache#4693)
444b12b 
* unify vm and interpreter objects

* move closure back vm

* adt/closure back to vm.adt/vm.closure

* closure base
alexwong pushed a commit to alexwong/tvm that referenced this pull request Feb 28, 2020
@zhiics @alexwong
[runtime][refactor] Unify vm and interpreter objects (apache#4693)
174ddbe 
* unify vm and interpreter objects

* move closure back vm

* adt/closure back to vm.adt/vm.closure

* closure base
zhiics added a commit to neo-ai/tvm that referenced this pull request Mar 2, 2020
@zhiics
[runtime][refactor] Unify vm and interpreter objects (apache#4693)
e51638d 
* unify vm and interpreter objects

* move closure back vm

* adt/closure back to vm.adt/vm.closure

* closure base
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jroesch jroesch jroesch left review comments

@MarisaKirisame MarisaKirisame MarisaKirisame approved these changes

@icemelon icemelon icemelon approved these changes

@tqchen tqchen tqchen approved these changes

@wweic wweic wweic approved these changes

Assignees

@wweic wweic

@jroesch jroesch

Labels
status: accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zhiics @tqchen @wweic @jroesch @MarisaKirisame @icemelon